sign: fix memory leaks and code cleanup

Return `const char *` instead of copy of the string -- this allow to
avoid unneeded copying and memory leaks in some constructions.
Minor code cleanup and optimisations.

Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index c3672b78b0bdb8760d00732e5d2b16d93975f3c6..b87f6c9045ae875e9d6206948a464810e161ca71 100644 (file)
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -1519,28 +1519,26 @@ ostree_verify_unwritten_commit (OtPullData                 *pull_data,
       gboolean ret = FALSE;
       g_autoptr(GBytes) signed_data = g_variant_get_data_as_bytes (commit);
       /* list all signature types in detached metadata and check if signed by any? */
-      g_auto(GStrv) names = ostree_sign_list_names();
+      g_auto (GStrv) names = ostree_sign_list_names();
       for (guint i=0; i < g_strv_length (names); i++)
         {
           g_autoptr (OstreeSign) sign = NULL;
+          g_autoptr (GError) local_error = NULL;
           g_autoptr (GVariant) signatures = NULL;
-          g_autofree gchar *signature_key = NULL;
-          g_autofree GVariantType *signature_format = NULL;
+          const gchar *signature_key = NULL;
+          GVariantType *signature_format = NULL;
           g_autofree gchar *pk_ascii = NULL;
           g_autofree gchar *pk_file = NULL;
 
-          if ((sign = ostree_sign_get_by_name (names[i], error)) == NULL)
-          {
-              g_clear_error (error);
-              continue;
-          }
+          if ((sign = ostree_sign_get_by_name (names[i], &local_error)) == NULL)
+            continue;
+
           signature_key = ostree_sign_metadata_key (sign);
           signature_format = (GVariantType *) ostree_sign_metadata_format (sign);
 
           signatures = g_variant_lookup_value (detached_metadata,
                                                signature_key,
                                                signature_format);
-
           if (!signatures)
             continue;
 
@@ -1558,8 +1556,8 @@ ostree_verify_unwritten_commit (OtPullData                 *pull_data,
               g_variant_builder_add (builder, "{sv}", "filename", g_variant_new_string (pk_file));
               options = g_variant_builder_end (builder);
 
-              if (!ostree_sign_load_pk (sign, options, error))
-                g_clear_error (error);
+              if (!ostree_sign_load_pk (sign, options, &local_error))
+                g_clear_error (&local_error);
             }
 
           /* Override key if it is set explicitly */
@@ -1583,27 +1581,23 @@ ostree_verify_unwritten_commit (OtPullData                 *pull_data,
                   pk = g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, key, key_len, sizeof(guchar));
                 }
 
-              if (!ostree_sign_set_pk (sign, pk, error))
-                g_clear_error (error);
+              if (!ostree_sign_set_pk (sign, pk, &local_error))
+                continue;
             }
 
           /* Set return to true if any sign fit */
           if (ostree_sign_metadata_verify (sign,
                                            signed_data,
                                            signatures,
-                                           error
+                                           &local_error
                                           ))
             ret = TRUE;
-          else
-            g_clear_error (error);
         }
 
       /* Mark the commit as verified to avoid double verification
        * see process_verify_result () for rationale */
       if (ret)
-        {
         g_hash_table_add (pull_data->verified_commits, g_strdup (checksum));
-        }
       else
         g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED,
                              "Can't verify commit");
@@ -1946,17 +1940,15 @@ scan_commit_object (OtPullData                 *pull_data,
       gboolean ret = FALSE;
       /* list all signature types in detached metadata and check if signed by any? */
       g_auto (GStrv) names = ostree_sign_list_names();
-      for (guint i=0; i < g_strv_length (names); i++)
+      for (char **iter=names; iter && *iter; iter++)
         {
           g_autoptr (OstreeSign) sign = NULL;
+          g_autoptr (GError) local_error = NULL;
           g_autofree gchar *pk_ascii = NULL;
           g_autofree gchar *pk_file = NULL;
 
-          if ((sign = ostree_sign_get_by_name (names[i], error)) == NULL)
-          {
-              g_clear_error (error);
-              continue;
-          }
+          if ((sign = ostree_sign_get_by_name (*iter, &local_error)) == NULL)
+            continue;
 
           /* Load keys for remote from file */
           ostree_repo_get_remote_option (pull_data->repo,
@@ -1972,8 +1964,8 @@ scan_commit_object (OtPullData                 *pull_data,
               g_variant_builder_add (builder, "{sv}", "filename", g_variant_new_string (pk_file));
               options = g_variant_builder_end (builder);
 
-              if (!ostree_sign_load_pk (sign, options, error))
-                g_clear_error (error);
+              if (!ostree_sign_load_pk (sign, options, &local_error))
+                g_clear_error (&local_error);
             }
 
           ostree_repo_get_remote_option (pull_data->repo,
@@ -1996,8 +1988,8 @@ scan_commit_object (OtPullData                 *pull_data,
                   pk = g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, key, key_len, sizeof(guchar));
                 }
 
-              if (!ostree_sign_set_pk (sign, pk, error))
-                g_clear_error (error);
+              if (!ostree_sign_set_pk (sign, pk, &local_error))
+                continue;
             }
 
 
@@ -2006,10 +1998,8 @@ scan_commit_object (OtPullData                 *pull_data,
                                          pull_data->repo,
                                          checksum,
                                          cancellable,
-                                         error))
+                                         &local_error))
             ret = TRUE;
-          else
-            g_clear_error (error);
 
         }
       if (!ret)

diff --git a/src/libostree/ostree-sign-dummy.c b/src/libostree/ostree-sign-dummy.c
index 34660cecb312bf37578ddb388b9e6687b79d775c..fb5a4f9e53559477f8c6fe426cf44430f929a622 100644 (file)
--- a/src/libostree/ostree-sign-dummy.c
+++ b/src/libostree/ostree-sign-dummy.c
@@ -108,30 +108,26 @@ gboolean ostree_sign_dummy_data (OstreeSign *self,
   return TRUE;
 }
 
-gchar * ostree_sign_dummy_get_name (OstreeSign *self)
+const gchar * ostree_sign_dummy_get_name (OstreeSign *self)
 {
   g_debug ("%s enter", __FUNCTION__);
   g_return_val_if_fail (OSTREE_IS_SIGN (self), FALSE);
 
-  g_autofree gchar *name = g_strdup(OSTREE_SIGN_DUMMY_NAME);
-
-  return g_steal_pointer (&name);
+  return OSTREE_SIGN_DUMMY_NAME;
 }
 
-gchar * ostree_sign_dummy_metadata_key (OstreeSign *self)
+const gchar * ostree_sign_dummy_metadata_key (OstreeSign *self)
 {
   g_debug ("%s enter", __FUNCTION__);
 
-  g_autofree gchar *key = g_strdup(OSTREE_SIGN_METADATA_DUMMY_KEY);
-  return g_steal_pointer (&key);
+  return OSTREE_SIGN_METADATA_DUMMY_KEY;
 }
 
-gchar * ostree_sign_dummy_metadata_format (OstreeSign *self)
+const gchar * ostree_sign_dummy_metadata_format (OstreeSign *self)
 {
   g_debug ("%s enter", __FUNCTION__);
 
-  g_autofree gchar *type = g_strdup(OSTREE_SIGN_METADATA_DUMMY_TYPE);
-  return g_steal_pointer (&type);
+  return OSTREE_SIGN_METADATA_DUMMY_TYPE;
 }
 
 gboolean ostree_sign_dummy_metadata_verify (OstreeSign *self,

diff --git a/src/libostree/ostree-sign-dummy.h b/src/libostree/ostree-sign-dummy.h
index 73bad135676b0051adc8db730603757078710e5b..847a7313ff5989d90bac43a986aee7382c9e43b7 100644 (file)
--- a/src/libostree/ostree-sign-dummy.h
+++ b/src/libostree/ostree-sign-dummy.h
@@ -39,7 +39,7 @@ G_DECLARE_FINAL_TYPE (OstreeSignDummy,
                       SIGN_DUMMY,
                       GObject)
 
-gchar * ostree_sign_dummy_get_name (OstreeSign *self);
+const gchar * ostree_sign_dummy_get_name (OstreeSign *self);
 
 gboolean ostree_sign_dummy_data (OstreeSign *self,
                                  GBytes *data,
@@ -47,8 +47,8 @@ gboolean ostree_sign_dummy_data (OstreeSign *self,
                                  GCancellable *cancellable,
                                  GError **error);
 
-gchar * ostree_sign_dummy_metadata_key (OstreeSign *self);
-gchar * ostree_sign_dummy_metadata_format (OstreeSign *self);
+const gchar * ostree_sign_dummy_metadata_key (OstreeSign *self);
+const gchar * ostree_sign_dummy_metadata_format (OstreeSign *self);
 
 gboolean ostree_sign_dummy_metadata_verify (OstreeSign *self,
                                             GBytes     *data,

diff --git a/src/libostree/ostree-sign-ed25519.c b/src/libostree/ostree-sign-ed25519.c
index 211e5572da5343b6802bc3a7aa743a583cd02f6d..c6c163025b9ed8e3a7a23307b02cde13d2796c1f 100644 (file)
--- a/src/libostree/ostree-sign-ed25519.c
+++ b/src/libostree/ostree-sign-ed25519.c
@@ -112,7 +112,7 @@ gboolean ostree_sign_ed25519_data (OstreeSign *self,
   OstreeSignEd25519 *sign = ostree_sign_ed25519_get_instance_private(OSTREE_SIGN_ED25519(self));
 
 #ifdef HAVE_LIBSODIUM
-  g_autofree guchar *sig = NULL;
+  guchar *sig = NULL;
 #endif
 
   if ((sign->initialized != TRUE) || (sign->secret_key == NULL))
@@ -137,37 +137,33 @@ gboolean ostree_sign_ed25519_data (OstreeSign *self,
       goto err;
     }
 
-  *signature = g_bytes_new (sig, sig_size);
+  *signature = g_bytes_new_take (sig, sig_size);
   return TRUE;
 #endif /* HAVE_LIBSODIUM */
 err:
   return FALSE;
 }
 
-gchar * ostree_sign_ed25519_get_name (OstreeSign *self)
+const gchar * ostree_sign_ed25519_get_name (OstreeSign *self)
 {
   g_debug ("%s enter", __FUNCTION__);
   g_return_val_if_fail (OSTREE_IS_SIGN (self), FALSE);
 
-  g_autofree gchar *name = g_strdup (OSTREE_SIGN_ED25519_NAME);
-
-  return g_steal_pointer (&name);
+  return OSTREE_SIGN_ED25519_NAME;
 }
 
-gchar * ostree_sign_ed25519_metadata_key (OstreeSign *self)
+const gchar * ostree_sign_ed25519_metadata_key (OstreeSign *self)
 {
   g_debug ("%s enter", __FUNCTION__);
 
-  g_autofree gchar *key = g_strdup(OSTREE_SIGN_METADATA_ED25519_KEY);
-  return g_steal_pointer (&key);
+  return OSTREE_SIGN_METADATA_ED25519_KEY;
 }
 
-gchar * ostree_sign_ed25519_metadata_format (OstreeSign *self)
+const gchar * ostree_sign_ed25519_metadata_format (OstreeSign *self)
 {
   g_debug ("%s enter", __FUNCTION__);
 
-  g_autofree gchar *type = g_strdup (OSTREE_SIGN_METADATA_ED25519_TYPE);
-  return g_steal_pointer (&type);
+  return OSTREE_SIGN_METADATA_ED25519_TYPE;
 }
 
 gboolean ostree_sign_ed25519_metadata_verify (OstreeSign *self,
@@ -187,7 +183,7 @@ gboolean ostree_sign_ed25519_metadata_verify (OstreeSign *self,
       g_set_error_literal (error,
                            G_IO_ERROR, G_IO_ERROR_FAILED,
                            "signature: ed25519: commit have no signatures of my type");
-      goto err;
+      goto out;
     }
 
   if (!g_variant_is_of_type (signatures, (GVariantType *) OSTREE_SIGN_METADATA_ED25519_TYPE))
@@ -195,14 +191,14 @@ gboolean ostree_sign_ed25519_metadata_verify (OstreeSign *self,
       g_set_error_literal (error,
                            G_IO_ERROR, G_IO_ERROR_FAILED,
                            "signature: ed25519: wrong type passed for verification");
-      goto err;
+      goto out;
     }
 
   if (sign->initialized != TRUE)
     {
       g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED,
                           "Not able to verify: libsodium library isn't initialized properly");
-      goto err;
+      goto out;
     }
 
 #ifdef HAVE_LIBSODIUM
@@ -217,7 +213,7 @@ gboolean ostree_sign_ed25519_metadata_verify (OstreeSign *self,
       options = g_variant_builder_end (builder);
 
       if (!ostree_sign_ed25519_load_pk (self, options, error))
-        goto err;
+        goto out;
     }
 
   g_debug ("verify: data hash = 0x%x", g_bytes_hash(data));
@@ -259,9 +255,8 @@ gboolean ostree_sign_ed25519_metadata_verify (OstreeSign *self,
                          "Not able to verify: no valid signatures found");
 #endif /* HAVE_LIBSODIUM */
 
+out:
   return ret;
-err:
-  return FALSE;
 }
 
 gboolean
@@ -312,7 +307,6 @@ gboolean ostree_sign_ed25519_set_sk (OstreeSign *self,
 
 #ifdef HAVE_LIBSODIUM
   OstreeSignEd25519 *sign = ostree_sign_ed25519_get_instance_private(OSTREE_SIGN_ED25519(self));
-  g_autofree char * hex = NULL;
 
   g_free (sign->secret_key);
 
@@ -326,9 +320,6 @@ gboolean ostree_sign_ed25519_set_sk (OstreeSign *self,
       goto err;
     }
 
-  hex = g_malloc0 (crypto_sign_SECRETKEYBYTES*2 + 1);
-//  g_debug ("Set ed25519 secret key = %s", sodium_bin2hex (hex, crypto_sign_SECRETKEYBYTES*2+1, sign->secret_key, n_elements));
-
   return TRUE;
 
 err:
@@ -348,7 +339,7 @@ gboolean ostree_sign_ed25519_set_pk (OstreeSign *self,
   /* Substitute the key(s) with a new one */
   if (sign->public_keys != NULL)
     {
-      g_list_free_full (sign->public_keys, g_object_unref);
+      g_list_free_full (sign->public_keys, g_free);
       sign->public_keys = NULL;
     }
 
@@ -380,9 +371,11 @@ gboolean ostree_sign_ed25519_add_pk (OstreeSign *self,
       goto err;
     }
 
-  key = g_memdup (key, n_elements);
   if (g_list_find (sign->public_keys, key) == NULL)
-      sign->public_keys = g_list_prepend (sign->public_keys, key);
+    {
+      gpointer newkey = g_memdup (key, n_elements);
+      sign->public_keys = g_list_prepend (sign->public_keys, newkey);
+    }
 
   return TRUE;
 
@@ -485,6 +478,7 @@ _load_pk_from_file (OstreeSign *self,
                     GError **error)
 {
   g_debug ("%s enter", __FUNCTION__);
+  g_debug ("Processing file '%s'", filename);
 
   g_autoptr (GFile) keyfile = NULL;
   g_autoptr (GFileInputStream) key_stream_in = NULL;
@@ -542,7 +536,7 @@ ostree_sign_ed25519_load_pk (OstreeSign *self,
   /* Clear already loaded keys */
   if (sign->public_keys != NULL)
     {
-      g_list_free_full (sign->public_keys, g_object_unref);
+      g_list_free_full (sign->public_keys, g_free);
       sign->public_keys = NULL;
     }
 

diff --git a/src/libostree/ostree-sign-ed25519.h b/src/libostree/ostree-sign-ed25519.h
index 797ac138093f2b91c94537e9fbf1eabffc8406c9..eb8f6701b0231d57d2d7ab783f56e8a1bd743434 100644 (file)
--- a/src/libostree/ostree-sign-ed25519.h
+++ b/src/libostree/ostree-sign-ed25519.h
@@ -46,9 +46,9 @@ gboolean ostree_sign_ed25519_data (OstreeSign *self,
                                  GCancellable *cancellable,
                                  GError **error);
 
-gchar * ostree_sign_ed25519_get_name (OstreeSign *self);
-gchar * ostree_sign_ed25519_metadata_key (OstreeSign *self);
-gchar * ostree_sign_ed25519_metadata_format (OstreeSign *self);
+const gchar * ostree_sign_ed25519_get_name (OstreeSign *self);
+const gchar * ostree_sign_ed25519_metadata_key (OstreeSign *self);
+const gchar * ostree_sign_ed25519_metadata_format (OstreeSign *self);
 
 gboolean ostree_sign_ed25519_metadata_verify (OstreeSign *self,
                                             GBytes     *data,

diff --git a/src/libostree/ostree-sign.c b/src/libostree/ostree-sign.c
index 5f7078f1175bb9277c3cda2371e2e57abfd744ba..6e67acaacf22e41e5fc310378597c57bac47aa0f 100644 (file)
--- a/src/libostree/ostree-sign.c
+++ b/src/libostree/ostree-sign.c
@@ -71,7 +71,7 @@ ostree_sign_default_init (OstreeSignInterface *iface)
   g_debug ("OstreeSign initialization");
 }
 
-gchar * ostree_sign_metadata_key (OstreeSign *self)
+const gchar * ostree_sign_metadata_key (OstreeSign *self)
 {
   g_debug ("%s enter", __FUNCTION__);
 
@@ -79,7 +79,7 @@ gchar * ostree_sign_metadata_key (OstreeSign *self)
   return OSTREE_SIGN_GET_IFACE (self)->metadata_key (self);
 }
 
-gchar * ostree_sign_metadata_format (OstreeSign *self)
+const gchar * ostree_sign_metadata_format (OstreeSign *self)
 {
   g_debug ("%s enter", __FUNCTION__);
 
@@ -134,7 +134,7 @@ ostree_sign_load_pk (OstreeSign *self,
   g_debug ("%s enter", __FUNCTION__);
 
   if (OSTREE_SIGN_GET_IFACE (self)->load_pk == NULL)
-    return FALSE;
+    return TRUE;
 
   return OSTREE_SIGN_GET_IFACE (self)->load_pk (self, options, error);
 }
@@ -170,8 +170,8 @@ ostree_sign_detached_metadata_append (OstreeSign *self,
 
   g_variant_dict_init (&metadata_dict, existing_metadata);
 
-  g_autofree gchar *signature_key = ostree_sign_metadata_key(self);
-  g_autofree GVariantType *signature_format = (GVariantType *) ostree_sign_metadata_format(self);
+  const gchar *signature_key = ostree_sign_metadata_key(self);
+  GVariantType *signature_format = (GVariantType *) ostree_sign_metadata_format(self);
 
   signature_data = g_variant_dict_lookup_value (&metadata_dict,
                                                 signature_key,
@@ -234,8 +234,8 @@ ostree_sign_commit_verify (OstreeSign     *self,
 
   g_autoptr(GVariant) signatures = NULL;
 
-  g_autofree gchar *signature_key = ostree_sign_metadata_key(self);
-  g_autofree GVariantType *signature_format = (GVariantType *) ostree_sign_metadata_format(self);
+  const gchar *signature_key = ostree_sign_metadata_key(self);
+  GVariantType *signature_format = (GVariantType *) ostree_sign_metadata_format(self);
 
   if (metadata)
     signatures = g_variant_lookup_value (metadata,

diff --git a/src/libostree/ostree-sign.h b/src/libostree/ostree-sign.h
index 78e2487d4f389b3bd02e288a1eb0b22fca5b14fa..a9648cb1de5953b25f0855f4b3d918271bc881e1 100644 (file)
--- a/src/libostree/ostree-sign.h
+++ b/src/libostree/ostree-sign.h
@@ -47,14 +47,14 @@ G_DECLARE_INTERFACE (OstreeSign, ostree_sign, OSTREE, SIGN, GObject)
 struct _OstreeSignInterface
 {
   GTypeInterface g_iface;
-  gchar *(* get_name) (OstreeSign *self);
+  const gchar *(* get_name) (OstreeSign *self);
   gboolean (* data)   (OstreeSign *self,
                        GBytes *data,
                        GBytes **signature,
                        GCancellable *cancellable,
                        GError **error);
-  gchar *(* metadata_key) (OstreeSign *self);
-  gchar *(* metadata_format) (OstreeSign *self);
+  const gchar *(* metadata_key) (OstreeSign *self);
+  const gchar *(* metadata_format) (OstreeSign *self);
   gboolean (* metadata_verify) (OstreeSign *self,
                                 GBytes *data,
                                 GVariant   *metadata,
@@ -90,10 +90,10 @@ gboolean ostree_sign_data (OstreeSign *self,
 
 
 _OSTREE_PUBLIC
-gchar * ostree_sign_metadata_key (OstreeSign *self);
+const gchar * ostree_sign_metadata_key (OstreeSign *self);
 
 _OSTREE_PUBLIC
-gchar * ostree_sign_metadata_format (OstreeSign *self);
+const gchar * ostree_sign_metadata_format (OstreeSign *self);
 
 _OSTREE_PUBLIC
 GVariant * ostree_sign_detached_metadata_append (OstreeSign *self,